-
Notifications
You must be signed in to change notification settings - Fork 95
Code snippets #759
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Code snippets #759
Conversation
ca7e301 to
998ac52
Compare
|
Thanks for opening the PR, @Edu92337! Just a heads-up that the current CI failures are unrelated to your changes. You can fix them by fetching updates from the main branch. You can either:
Once that’s done, the CI should pass again (or fail for reasons related to your PR). The same also applies to your other PRs #765 and #757. Since you currently have 3 PRs awaiting review, please hold off on opening new ones until we've merged at least some of the existing work. This helps us in two ways:
This is not a criticism of your work by the way, the effort you've put into these PRs is appreciated. This is purely about managing our review capacity effectively. |
|
Thanks for the heads-up. I've updated all three PRs with the latest main branch. I'll wait for your review before opening any new PRs. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #759 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 34 34
Lines 2111 2111
=========================================
Hits 2111 2111 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
sfmig
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @Edu92337, thanks for having a go at this issue.
I like the "Verifying sample data" section you drafted. However, I noticed that it is added to the user guide, rather than to the Contributing guide as was originally suggested in the issue. I was wondering if there was a reason? My original suggestion was to include this code snippet + brief text just after the CONTRIBUTION > Adding data section, so that we can quickly check if the data was correctly added to the GIN repository. But let me know if you think otherwise. If you agree, would you mind moving it to the Contributing guide?
Re the added docstring example in the sample_data.py module, I think we should remove it for consistency with the rest of the codebase. I think in all cases we use the examples in the docstrings to demo the function they refer to, and not any other, just for clarity for our users.
Let me know thoughts, thanks again!
|
thanks for the feedback @sfmig , I misunderstood where this should go. I'll move the "Verifying sample data" section to the Contributing guide and remove the example from the docstring to keep it consistent with the rest of the codebase. |
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
|



Description
What is this PR
Why is this PR needed?
Users need a quick way to verify that sample datasets can be fetched and loaded correctly. Currently, there's no documentation showing this workflow, which can leave users uncertain if their data loading is working properly.
What does this PR do?
Adds code snippets to the documentation demonstrating how to verify sample data loading:
fetch_dataset_paths()docstring with a verification exampleReferences
Closes #654
How has this PR been tested?
Is this a breaking change?
No. This PR only adds documentation and does not modify any existing code functionality.
Does this PR require an update to the documentation?
This PR is a documentation update. No further documentation changes are needed.
Checklist: